Skip to content

Improve container deployment UX#3

Draft
Colossus14 wants to merge 4 commits into
NextronSystems:masterfrom
Colossus14:ux-hardening-container-guide
Draft

Improve container deployment UX#3
Colossus14 wants to merge 4 commits into
NextronSystems:masterfrom
Colossus14:ux-hardening-container-guide

Conversation

@Colossus14

@Colossus14 Colossus14 commented Jun 5, 2026

Copy link
Copy Markdown

Draft note: this PR is intentionally marked as draft. It captures an initial UX-review patch, but the findings and direction should be discussed before treating it as ready for review or merge.

This PR addresses the first-run friction I hit while reviewing the container deployment guide.

Findings covered:

  • Running the published image without CONTRACT_TOKEN prints an error but exits 0, so automation can treat a failed first start as success.
  • Quick start puts the token inline on the command line; using an .env file is easier to repeat and avoids shell-history leakage.
  • The guide did not show a simple post-start validation path, so an operator has to guess whether to check logs, ps output, or an API endpoint.
  • Existing collectors may still point at the old service port, while the container defaults to 8080; the guide now calls out either updating collectors or overriding PORT.
  • LOG_ENABLED and VFS_ENABLED said to uncomment a volume below, but no matching volume mounts were present.
  • THOR_ARGS and THUNDERSTORM_ARGS were easy to confuse; the guide now distinguishes service arguments from scan arguments.
  • TLS wording mentioned Docker secrets, while the Compose template actually shows read-only bind mounts.
  • Cleanup behavior was not documented, especially the difference between stopping the service and deleting downloaded THOR state.

Validation:

  • sh -n entrypoint.sh
  • parsed docker-compose.yml with Python YAML
  • built the image with Podman on rune
  • verified the no-token startup path now exits 1 instead of 0

Note: Docker was not installed on the workstation used for review; Podman on rune was used for the build/run validation.

@Colossus14 Colossus14 marked this pull request as draft June 5, 2026 07:55

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This draft PR improves the first-run/operator experience for deploying Thunderstorm via the published container image by tightening failure behavior and expanding the deployment guide with more repeatable, safer steps.

Changes:

  • Update the Quick-Start to use a .env file, add post-start validation steps, and document cleanup/troubleshooting paths.
  • Make entrypoint.sh fail with non-zero exit codes on missing token and other initialization failures, and add a clearer download failure message.
  • Extend the Compose template with optional persistent mounts for logs/VFS data and allow CONTRACT_TOKEN to be omitted (when THOR is already present in the volume).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
README.md Adds .env-based Quick-Start, validation commands, troubleshooting/cleanup guidance, and clarifies configuration behaviors.
entrypoint.sh Ensures missing token/version-detection failures exit non-zero; adds diagnostics for download failures.
docker-compose.yml Allows empty CONTRACT_TOKEN expansion; documents optional log/VFS persistence via additional volumes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread entrypoint.sh Outdated
Comment on lines 16 to 17
unzip -o -q "$TEMP_DIR/thor.zip" -d "$TARGET_DIR" && \
rm "$TEMP_DIR/thor.zip"
Comment thread entrypoint.sh Outdated
Comment on lines +22 to +24
echo "THOR binary not found at $TARGET_DIR/thor-linux-64. Abort!"
echo "Please verify that your CONTRACT_TOKEN is set and valid."
exit 0
echo "Please verify that your CONTRACT_TOKEN is set and valid, and that the contract has an issued Thunderstorm license."
exit 1
Comment thread README.md Outdated
Comment on lines +78 to +80
THOR signatures are updated automatically on every container start. To keep them fresh without manual restarts, set `SIGNATURE_UPDATE_INTERVAL` (in hours) to schedule recurring updates.

The update mechanism depends on the THOR major version. On THOR 10, new signatures only take effect after a restart. Docker's health check therefore marks the container as unhealthy once `SIGNATURE_UPDATE_INTERVAL` has elapsed, prompting Docker to restart it. The new signatures are then fetched as part of the regular container start, at the cost of a brief API downtime. THOR 11 uses Thunderstorm's built-in signature-update feature to download and apply signatures in-place, leaving the API available throughout.
The update mechanism depends on the THOR major version. On THOR 10, new signatures only take effect after a restart. Docker's health check therefore stops the container once `SIGNATURE_UPDATE_INTERVAL` has elapsed and the restart policy brings it back. The new signatures are then fetched as part of the regular container start, at the cost of a brief API downtime. THOR 11 uses Thunderstorm's built-in signature-update feature to download and apply signatures in-place, leaving the API available throughout.
- entrypoint.sh: check unzip and remove the archive on both success and
  failure so a corrupt/partial download fails with exit 1 instead of
  falling through to a misleading "THOR binary not found".
- entrypoint.sh: write the fatal "THOR binary not found" messages to
  stderr for consistency and easier log parsing.
- Document the per-version signature-update defaults (THOR 11 refreshes
  in-process every 24h by default; THOR 10 has no periodic refresh unless
  SIGNATURE_UPDATE_INTERVAL is set) in README.md and docker-compose.yml.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants